Review Request: Less repainting on mousePressEvent(), moseReleaseEvent() and mouseDoubleClickEvent() in FolderView::IconView

Ignat Semenov 13thguards at gmail.com
Sun Feb 5 19:38:43 UTC 2012



> On Feb. 5, 2012, 6:46 p.m., Fredrik Höglund wrote:
> > Aside from some minor nitpicks, it looks good.

I forgot to add that I couldn't optimize the right click path in mousePressEvent(). The problem is that in order to get the containment context menu, the RMB press event is propagated to the parent and triggers an unconditional repaint. I confirmed this behavior by removing all the markAreaDirty() calls in that path - then view still repainted fine. Thus, a right click repaints the whole view, voiding the optimization of that code path. Any ideas?

The optimization in that branch is there just for the sake completeness - the view will repaint fully anyway.


> On Feb. 5, 2012, 6:46 p.m., Fredrik Höglund wrote:
> > plasma/applets/folderview/iconview.cpp, line 1803
> > <http://git.reviewboard.kde.org/r/103822/diff/1/?file=48118#file48118line1803>
> >
> >     Make these const. There is also a whitespace error on this line.

Sorry, my fault. I will make a big poster "const correctness and git diff --check" and place it on the wall above the desk  ;)


> On Feb. 5, 2012, 6:46 p.m., Fredrik Höglund wrote:
> > plasma/applets/folderview/iconview.cpp, line 2605
> > <http://git.reviewboard.kde.org/r/103822/diff/1/?file=48118#file48118line2605>
> >
> >     Change the name of this variable to 'rect'.

Indeed, it is not necessarily dirty. Copy-paste :)


- Ignat


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


On Jan. 29, 2012, 3:54 p.m., Ignat Semenov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103822/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2012, 3:54 p.m.)
> 
> 
> Review request for Plasma, Aaron J. Seigo and Marco Martin.
> 
> 
> Description
> -------
> 
> This patch aims to save some repaints in FolderView::IconView on the various mouseEvent()'s by choosing what to repaint in a bit smarter way.
> 
> 
> Diffs
> -----
> 
>   plasma/applets/folderview/iconview.h 66ccb98 
>   plasma/applets/folderview/iconview.cpp 5b0cd98 
> 
> Diff: http://git.reviewboard.kde.org/r/103822/diff/diff
> 
> 
> Testing
> -------
> 
> Testing done against master, seems to behave indentically.
> 
> 
> Thanks,
> 
> Ignat Semenov
> 
>

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


More information about the Plasma-devel mailing list