Review Request 126466: Drop unneeded timer from PlasmaCore.IconItem

Eike Hein hein at kde.org
Tue Dec 22 17:24:56 UTC 2015



> On Dec. 22, 2015, 5:13 p.m., David Edmundson wrote:
> > src/declarativeimports/core/iconitem.cpp, line 253
> > <https://git.reviewboard.kde.org/r/126466/diff/4/?file=425385#file425385line253>
> >
> >     given you have an explicit call to polish() I don't think we need the guard?

Agreed


> On Dec. 22, 2015, 5:13 p.m., David Edmundson wrote:
> > src/declarativeimports/core/iconitem.cpp, line 332
> > <https://git.reviewboard.kde.org/r/126466/diff/4/?file=425385#file425385line332>
> >
> >     does this not need an update() too?

No, the update() calls are back unchanged in loadPixmap() now - I don't like them, but on the flip side it doesn't depart from (theoretically) tested-code someone thought about. Calls to update() don't seem to always call updatePolish() if no polish event has been scheduled, though - I tried doing polish implicitly from scheduled updates first and it never ran.


- Eike


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


On Dec. 22, 2015, 4:58 p.m., Eike Hein wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126466/
> -----------------------------------------------------------
> 
> (Updated Dec. 22, 2015, 4:58 p.m.)
> 
> 
> Review request for Plasma and David Edmundson.
> 
> 
> Bugs: 345545
>     http://bugs.kde.org/show_bug.cgi?id=345545
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> This timer seems to exist purely to delay loading the pixmap for the purpose of "hey, maybe it will work later ..." when it's not actually needed thanks to QQmlParserStatus. Dropping it brings speed up to par with QIconItem and fixes various icon flicker around Plasma, in particular in the Task Manager: When swapping out a launcher delegate for a task delegate, or when swapping out delegates during virtual desktop switches.
> 
> 
> Diffs
> -----
> 
>   src/declarativeimports/core/iconitem.h 3abef40 
>   src/declarativeimports/core/iconitem.cpp bada940 
> 
> Diff: https://git.reviewboard.kde.org/r/126466/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Eike Hein
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20151222/db7ae708/attachment.html>


More information about the Plasma-devel mailing list