----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122861/#review77202 -----------------------------------------------------------
Looks good to me. It would be good if Eike could also have a look since he's refactoring this code right now, and his refactoring might benefit from the same changes, if not avoid conflicts. One stylistic remark (spread over three lines) inline. containments/desktop/package/contents/code/LayoutManager.js <https://git.reviewboard.kde.org/r/122861/#comment53030> I think it's more readable to init j before the loop. containments/desktop/package/contents/code/LayoutManager.js <https://git.reviewboard.kde.org/r/122861/#comment53031> same here containments/desktop/package/contents/code/LayoutManager.js <https://git.reviewboard.kde.org/r/122861/#comment53032> init j before loop for readability - Sebastian Kügler On March 8, 2015, 9:08 p.m., Kai Uwe Broulik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122861/ > ----------------------------------------------------------- > > (Updated March 8, 2015, 9:08 p.m.) > > > Review request for Plasma. > > > Repository: plasma-desktop > > > Description > ------- > > This micro-optimizes the LayoutManager by: > - using Array and Object literals rather than new Object/Array, and also > creating the whole structure at once if applicable > - Store end values in for loops rather than calculating them on each iteration > - Adjust coding style here and there > > > Diffs > ----- > > containments/desktop/package/contents/code/LayoutManager.js 14d0dfc > > Diff: https://git.reviewboard.kde.org/r/122861/diff/ > > > Testing > ------- > > Moving applets (especially using Eike's press-and-hold when using it on a > touchscreen :P) feels snappier, doesn't print any new warnings on console and > seems to work as before. However, I can no longer cause plasmashell to go > berserk when moving a small applet ontop of a huge one (eg. small sticky note > on wide fuzzy clock) where it desperately tries to find a place and fails. > > I think the grid size should be based on units somehow, having a 24x24 grid > on a high dpi screen also benefits the aforementioned behavior. > > One surprising discovery I made is that using Qt.point instead of a > handcrafted JS Object is one order of magnitude(!) slower. > > > Thanks, > > Kai Uwe Broulik > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel