----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128821/#review98864 -----------------------------------------------------------
I don't have a touch screen to test with, unfortunately. ui/pageview.cpp (line 1490) <https://git.reviewboard.kde.org/r/128821/#comment66563> always {} ui/pageview.cpp (line 1500) <https://git.reviewboard.kde.org/r/128821/#comment66559> Don't put assignments in the if condition. Also, prefer to do an early exit, i. e. if (!pinchGesture) { return false; } or similar. Lastly, don't use static_cast here, it doesn't do runtime type checking. And always prefer qobject_cast over dynamic_cast if you can (that is, if you're casting QObjects). It is faster than dynamic_cast and works across dynamic libraries. ui/pageview.cpp (line 1505) <https://git.reviewboard.kde.org/r/128821/#comment66562> potentially uninitialized, add some default value. ui/pageview.cpp (line 1507) <https://git.reviewboard.kde.org/r/128821/#comment66561> always use {}, even for oneliners. ui/pageview.cpp (line 1510) <https://git.reviewboard.kde.org/r/128821/#comment66560> Why put this in a separate variable? - Martin Tobias Holmedahl Sandsmark On Sept. 3, 2016, 5:49 p.m., Oliver Sander wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128821/ > ----------------------------------------------------------- > > (Updated Sept. 3, 2016, 5:49 p.m.) > > > Review request for Okular. > > > Bugs: 366781 > http://bugs.kde.org/show_bug.cgi?id=366781 > > > Repository: okular > > > Description > ------- > > The attached patch implements document zoom controlled by a two-finger pinch > gesture on a touch screen. > > The actual zooming was done by copy'n'paste, I don't understand the full > details of how zooming works in okular, yet. So a bit of review would be > appreciated. > > > Diffs > ----- > > ui/pageview.h 01c39bf > ui/pageview.cpp cd53407 > > Diff: https://git.reviewboard.kde.org/r/128821/diff/ > > > Testing > ------- > > I tested it on my Lenovo Thinkpad Yoga, and it works nicely there. > > > Thanks, > > Oliver Sander > >
