Review Request: use Plasma::Dialog for brightness osd

[Xuetian Weng]

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

Review request for kde-workspace, Plasma and Aaron J. Seigo.


Description
---

well, you know.. this would fix the shadow problem.


Diffs
-

  powerdevil/daemon/brightnessosdwidget.h ef08903 
  powerdevil/daemon/brightnessosdwidget.cpp e4cf80a 

Diff: http://git.reviewboard.kde.org/r/108222/diff/


Testing
---

locally tested, problem.


Thanks,

Xuetian Weng

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

Re: Review Request: use Plasma::Dialog for brightness osd

[Xuetian Weng]

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

(Updated Jan. 6, 2013, 1:36 p.m.)


Review request for kde-workspace, Plasma and Aaron J. Seigo.


Description (updated)
---

well, you know.. this would fix the shadow problem, and clean up code actually.


Diffs
-

  powerdevil/daemon/brightnessosdwidget.h ef08903 
  powerdevil/daemon/brightnessosdwidget.cpp e4cf80a 

Diff: http://git.reviewboard.kde.org/r/108222/diff/


Testing (updated)
---

locally tested, no problem.


Thanks,

Xuetian Weng

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

Re: Review Request: use Plasma::Dialog for brightness osd

[Kai Uwe Broulik]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108222/#review24834
---


The code basically is fine, except that you could make the OSD a bit wider 
(increase width) like the old one.

There is a problem on the Plasma side, however. I adapted your code for KMix 
which also suffers the same problem, and in KMix it works perfectly.
In PowerDevil, however, I see no change, I get a fully-opaque white background 
dialog with no shadows. I guess this is because PowerDevil starts quite early 
in the startup process and so Plasma (and probably the compositor) is not up 
and running. Seems that broken detection when compositing state changes 
(https://bugs.kde.org/show_bug.cgi?id=179042) is the cause.

- Kai Uwe Broulik


On Jan. 6, 2013, 1:36 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108222/
> ---
> 
> (Updated Jan. 6, 2013, 1:36 p.m.)
> 
> 
> Review request for kde-workspace, Plasma and Aaron J. Seigo.
> 
> 
> Description
> ---
> 
> well, you know.. this would fix the shadow problem, and clean up code 
> actually.
> 
> 
> Diffs
> -
> 
>   powerdevil/daemon/brightnessosdwidget.h ef08903 
>   powerdevil/daemon/brightnessosdwidget.cpp e4cf80a 
> 
> Diff: http://git.reviewboard.kde.org/r/108222/diff/
> 
> 
> Testing
> ---
> 
> locally tested, no problem.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Re: Review Request: use Plasma::Dialog for brightness osd

[Xuetian Weng]

> On Jan. 6, 2013, 3:25 p.m., Kai Uwe Broulik wrote:
> > The code basically is fine, except that you could make the OSD a bit wider 
> > (increase width) like the old one.
> > 
> > There is a problem on the Plasma side, however. I adapted your code for 
> > KMix which also suffers the same problem, and in KMix it works perfectly.
> > In PowerDevil, however, I see no change, I get a fully-opaque white 
> > background dialog with no shadows. I guess this is because PowerDevil 
> > starts quite early in the startup process and so Plasma (and probably the 
> > compositor) is not up and running. Seems that broken detection when 
> > compositing state changes (https://bugs.kde.org/show_bug.cgi?id=179042) is 
> > the cause.

Well.. at least you can test with:
kquitapp kded; sleep 1; kded4

To see if we don't consider that bug, it will work or not.


- Xuetian


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108222/#review24834
---


On Jan. 6, 2013, 1:36 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108222/
> ---
> 
> (Updated Jan. 6, 2013, 1:36 p.m.)
> 
> 
> Review request for kde-workspace, Plasma and Aaron J. Seigo.
> 
> 
> Description
> ---
> 
> well, you know.. this would fix the shadow problem, and clean up code 
> actually.
> 
> 
> Diffs
> -
> 
>   powerdevil/daemon/brightnessosdwidget.h ef08903 
>   powerdevil/daemon/brightnessosdwidget.cpp e4cf80a 
> 
> Diff: http://git.reviewboard.kde.org/r/108222/diff/
> 
> 
> Testing
> ---
> 
> locally tested, no problem.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Re: Review Request: use Plasma::Dialog for brightness osd

[Kai Uwe Broulik]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108222/#review24840
---


Just applied the patch from Review 107983 and your patch resolves the issue. 
Would you mind if I use that code to fix KMix OSD?


powerdevil/daemon/brightnessosdwidget.cpp


Originally the meterWidth was iconSize * 12 resulting in the total width of 
(roughly):

iconSize * 13 + m_volumeLabel->nativeWidget()->size().width();


- Kai Uwe Broulik


On Jan. 6, 2013, 1:36 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108222/
> ---
> 
> (Updated Jan. 6, 2013, 1:36 p.m.)
> 
> 
> Review request for kde-workspace, Plasma and Aaron J. Seigo.
> 
> 
> Description
> ---
> 
> well, you know.. this would fix the shadow problem, and clean up code 
> actually.
> 
> 
> Diffs
> -
> 
>   powerdevil/daemon/brightnessosdwidget.h ef08903 
>   powerdevil/daemon/brightnessosdwidget.cpp e4cf80a 
> 
> Diff: http://git.reviewboard.kde.org/r/108222/diff/
> 
> 
> Testing
> ---
> 
> locally tested, no problem.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Re: Review Request: use Plasma::Dialog for brightness osd

[Xuetian Weng]

> On Jan. 6, 2013, 3:45 p.m., Kai Uwe Broulik wrote:
> > Just applied the patch from Review 107983 and your patch resolves the 
> > issue. Would you mind if I use that code to fix KMix OSD?

Well.. actually I have a review for kmix :P
https://git.reviewboard.kde.org/r/108223/


- Xuetian


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108222/#review24840
---


On Jan. 6, 2013, 1:36 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108222/
> ---
> 
> (Updated Jan. 6, 2013, 1:36 p.m.)
> 
> 
> Review request for kde-workspace, Plasma and Aaron J. Seigo.
> 
> 
> Description
> ---
> 
> well, you know.. this would fix the shadow problem, and clean up code 
> actually.
> 
> 
> Diffs
> -
> 
>   powerdevil/daemon/brightnessosdwidget.h ef08903 
>   powerdevil/daemon/brightnessosdwidget.cpp e4cf80a 
> 
> Diff: http://git.reviewboard.kde.org/r/108222/diff/
> 
> 
> Testing
> ---
> 
> locally tested, no problem.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Re: Review Request: use Plasma::Dialog for brightness osd

[Xuetian Weng]

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

(Updated Jan. 6, 2013, 10:25 p.m.)


Review request for kde-workspace, Plasma and Aaron J. Seigo.


Changes
---

change the size


Description
---

well, you know.. this would fix the shadow problem, and clean up code actually.


Diffs (updated)
-

  powerdevil/daemon/brightnessosdwidget.h ef08903 
  powerdevil/daemon/brightnessosdwidget.cpp e4cf80a 

Diff: http://git.reviewboard.kde.org/r/108222/diff/


Testing
---

locally tested, no problem.


Thanks,

Xuetian Weng

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

Re: Review Request: use Plasma::Dialog for brightness osd

[Kai Uwe Broulik]

> On Jan. 6, 2013, 3:45 p.m., Kai Uwe Broulik wrote:
> > Just applied the patch from Review 107983 and your patch resolves the 
> > issue. Would you mind if I use that code to fix KMix OSD?
> 
> Xuetian Weng wrote:
> Well.. actually I have a review for kmix :P
> https://git.reviewboard.kde.org/r/108223/

Haven't seen it :) Looks good from my POV but add the solid group because these 
are the ones responsible for PowerDevil :)


- Kai Uwe


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108222/#review24840
---


On Jan. 6, 2013, 10:25 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108222/
> ---
> 
> (Updated Jan. 6, 2013, 10:25 p.m.)
> 
> 
> Review request for kde-workspace, Plasma and Aaron J. Seigo.
> 
> 
> Description
> ---
> 
> well, you know.. this would fix the shadow problem, and clean up code 
> actually.
> 
> 
> Diffs
> -
> 
>   powerdevil/daemon/brightnessosdwidget.h ef08903 
>   powerdevil/daemon/brightnessosdwidget.cpp e4cf80a 
> 
> Diff: http://git.reviewboard.kde.org/r/108222/diff/
> 
> 
> Testing
> ---
> 
> locally tested, no problem.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Re: Review Request: use Plasma::Dialog for brightness osd

[Xuetian Weng]

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

(Updated Jan. 6, 2013, 11:53 p.m.)


Review request for kde-workspace, Plasma, Solid, and Aaron J. Seigo.


Description
---

well, you know.. this would fix the shadow problem, and clean up code actually.


Diffs
-

  powerdevil/daemon/brightnessosdwidget.h ef08903 
  powerdevil/daemon/brightnessosdwidget.cpp e4cf80a 

Diff: http://git.reviewboard.kde.org/r/108222/diff/


Testing
---

locally tested, no problem.


Thanks,

Xuetian Weng

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

Re: Review Request: use Plasma::Dialog for brightness osd

[Aaron J. Seigo]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108222/#review24870
---

Ship it!


Ship It!

- Aaron J. Seigo


On Jan. 6, 2013, 11:53 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108222/
> ---
> 
> (Updated Jan. 6, 2013, 11:53 p.m.)
> 
> 
> Review request for kde-workspace, Plasma, Solid, and Aaron J. Seigo.
> 
> 
> Description
> ---
> 
> well, you know.. this would fix the shadow problem, and clean up code 
> actually.
> 
> 
> Diffs
> -
> 
>   powerdevil/daemon/brightnessosdwidget.h ef08903 
>   powerdevil/daemon/brightnessosdwidget.cpp e4cf80a 
> 
> Diff: http://git.reviewboard.kde.org/r/108222/diff/
> 
> 
> Testing
> ---
> 
> locally tested, no problem.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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

Re: Review Request: use Plasma::Dialog for brightness osd

[Commit Hook]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108222/#review24913
---


This review has been submitted with commit 
d2cd9b46c506063fbd1520927e5c6d18c66e505f by Weng Xuetian to branch KDE/4.10.

- Commit Hook


On Jan. 6, 2013, 11:53 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108222/
> ---
> 
> (Updated Jan. 6, 2013, 11:53 p.m.)
> 
> 
> Review request for kde-workspace, Plasma, Solid, and Aaron J. Seigo.
> 
> 
> Description
> ---
> 
> well, you know.. this would fix the shadow problem, and clean up code 
> actually.
> 
> 
> Diffs
> -
> 
>   powerdevil/daemon/brightnessosdwidget.h ef08903 
>   powerdevil/daemon/brightnessosdwidget.cpp e4cf80a 
> 
> Diff: http://git.reviewboard.kde.org/r/108222/diff/
> 
> 
> Testing
> ---
> 
> locally tested, no problem.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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